Skip to content

Conversation

@joshheald
Copy link
Contributor

Description

This test has been around for about a year, and there's no recent changes in the area, so I'm not really sure why it's started getting flaky... I also couldn't reproduce it reliably even running the tests thousands of times.

I couldn't see anything obvious, so this is a fairly speculative improvement, but removing the sleep should help.

Test Steps

In Xcode, right click the test suite diamond and select Run Tests repeatedly and run them 1000 times. I tried 5000 with no failures.

Screenshots

CleanShot 2025-11-07 at 12 40 25@2x
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@joshheald joshheald added this to the 23.7 milestone Nov 7, 2025
@joshheald joshheald requested a review from itsmeichigo November 7, 2025 12:45
@joshheald joshheald added the type: task An internally driven task. label Nov 7, 2025
@wpmobilebot
Copy link
Collaborator

App Icon📲 You can test the changes from this Pull Request in WooCommerce iOS Prototype by scanning the QR code below to install the corresponding build.

App NameWooCommerce iOS Prototype
Build Numberpr16330-0905082
Version23.6
Bundle IDcom.automattic.alpha.woocommerce
Commit0905082
Installation URL3oaq27m1mlkvg
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

Copy link
Contributor

@itsmeichigo itsmeichigo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I followed the instruction to run the test 1000 times. It failed in iteration 161 for me:

✘ Test syncOrder_when_already_syncing_doesnt_call_orderService() recorded an issue at PointOfSaleOrderControllerTests.swift:91:9: Expectation failed: (mockOrderService.syncOrderWasCalled → false) == true

Since I don't have enough context on this area, I hope @iamgabrielma can help take a look? 🙏

@joshheald
Copy link
Contributor Author

Observation is a really good framework for normal use, and kinda bad for testing. Unless anyone has any specific ideas to improve it, I'd vote for removing the test over spending more time on fixing it.

@iamgabrielma iamgabrielma self-assigned this Nov 11, 2025
Copy link
Contributor

@iamgabrielma iamgabrielma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes LGTM. Interestingly running that specific test 1000 times failed once, while running the whole suite 1000 times (25k tests) never failed.

✔ Test run with 25000 tests passed after 35.358 seconds.

For a 0.1% chance I'd keep it, if this becomes a CI issue and starts to happen often I'd say let's just remove it.

@joshheald joshheald merged commit 10c06f8 into trunk Nov 11, 2025
18 of 19 checks passed
@joshheald joshheald deleted the task/fix-flaky-ordersync-test branch November 11, 2025 04:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: task An internally driven task.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants